[FSSDK-11170] update: decision service methods for cmab #583

FarhanAnjum-opti · 2025-09-24T15:46:52Z

Summary

Decision Service methods to handle CMAB

Test plan

Added unit tests

Issues

FSSDK-11170

…vice

…e to support CMAB UUID handling

…and include CMAB UUIDs in responses

…ils for CMAB configuration

…ations over CMAB service decisions in DecisionService

…rvice failures

…information

…tly verify error state

…izely and DecisionService

…out CMAB logic

…text and related fetcher classes

…serContextTest

… documentation

muzahidul-opti

Changes look good to me. Added few comments.

core-api/src/main/java/com/optimizely/ab/Optimizely.java

core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java

core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java

core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java

core-api/src/main/java/com/optimizely/ab/optimizelydecision/AsyncDecisionsFetcher.java

…aking logic

…ecision responses

…ent key retrieval

…atureDecision

…eation in CMAB client

…hance decision handling

…lientHelper

core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientHelper.java

muzahidul-opti

LGTM

jaeopt

A few changes in API definitions. Can you refactor them first?
I need more time to review the cmab logic.

jaeopt · 2025-10-08T17:36:16Z

core-api/src/main/java/com/optimizely/ab/Optimizely.java

            String flagKey = flagsWithoutForcedDecision.get(i).getKey();
+
+            if (error) {
+                OptimizelyDecision optimizelyDecision = OptimizelyDecision.newErrorDecision(flagKey, user, DecisionMessage.CMAB_ERROR.reason(experimentKey));


Do we always report CMAB error for any decision errors? Is this safe?

As far as I understand, we get error from decision service only when cmab fails. So this error flag is only true for cmab errors. @raju-opti can verify.

core-api/src/main/java/com/optimizely/ab/Optimizely.java

core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

… backward compatibility for mobile apps

… decisions in OptimizelyUserContext

…rContextTest for consistency

…od for consistency

…ecision handling

jaeopt

A few more suggestions at the top level.

core-api/src/main/java/com/optimizely/ab/Optimizely.java

core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java

core-api/src/main/java/com/optimizely/ab/Optimizely.java

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

core-api/src/main/java/com/optimizely/ab/Optimizely.java

…larity and maintainability

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

…ckage-private and add copyright notice to DecisionPath

…and OptimizelyUserContext with corresponding tests

…ndency and adjust related tests

…and enhance builder methods for cache configuration

jaeopt

I see it's now more aligned with android-sdk. I added more comments. Let's discuss.

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

jaeopt · 2025-10-28T00:46:00Z

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

+
            if (decision != null) {
-                decisions.add(new DecisionResponse(decision, reasons));
+                decisions.add(new DecisionResponse(decision, reasons, error, decision.cmabUUID));


Do we have any case we need pass cmabUUID separately though it's already in decision?

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java

core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

…nService and adjust tests accordingly

…ervice to utilize it

…djust instance creation logic

…on and enhance cache handling logging

jaeopt

Bucket cleaning up looks good! A few more changes suggested.

jaeopt · 2025-11-07T23:35:56Z

core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java

+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Matchers.eq;


these changes not used? clean up.

jaeopt · 2025-11-07T23:43:06Z

core-api/src/main/java/com/optimizely/ab/internal/CacheWithRemove.java

+ */
+package com.optimizely.ab.internal;
+
+public interface CacheWithRemove<T> extends Cache<T> {


java supports default implementation for interface -
We can remove CacheWithRemove and add remove() to Cache, so backward compatible with existing Cache

default void remove(String key) { // Default implementation does nothing // Implementations should override this method to provide actual removal functionality }

jaeopt · 2025-11-07T23:44:50Z

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

 public class DefaultCmabService implements CmabService {
-
-    private final DefaultLRUCache<CmabCacheValue> cmabCache;
+    public static final int DEFAULT_CMAB_CACHE_SIZE = 1000;


Suggested change

public static final int DEFAULT_CMAB_CACHE_SIZE = 1000;

public static final int DEFAULT_CMAB_CACHE_SIZE = 10000;

Should be consistent with other server-sdks

jaeopt · 2025-11-07T23:45:40Z

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

-
-    private final DefaultLRUCache<CmabCacheValue> cmabCache;
+    public static final int DEFAULT_CMAB_CACHE_SIZE = 1000;
+    public static final int DEFAULT_CMAB_CACHE_TIMEOUT_SECS = 300; // 5 minutes


Suggested change

public static final int DEFAULT_CMAB_CACHE_TIMEOUT_SECS = 300; // 5 minutes

public static final int DEFAULT_CMAB_CACHE_TIMEOUT_SECS = 30*60; // 30 minutes

Should be consistent with other server-sdks

jaeopt · 2025-11-07T23:46:52Z

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

+        /**
+         * Set the timeout duration for cached CMAB decisions.
+         * 
+         * Default value is 300 seconds (5 minutes).


Fix this comment too

jaeopt · 2025-11-07T23:47:01Z

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

+        /**
+         * Set the maximum size of the CMAB cache.
+         * 
+         * Default value is 1000 entries.


Fxi this comment too

FarhanAnjum-opti added 17 commits September 19, 2025 16:18

update: add CmabService to Optimizely class and builder

26e6393

update: integrate CMAB service into OptimizelyFactory

ad63201

update: change CmabService field to non-nullable in Optimizely class

fbed362

update: add CmabService to DecisionService and its tests

53d754a

update: implement CMAB traffic allocation in Bucketer and DecisionSer…

9905026

…vice

update: enhance DecisionService, FeatureDecision, and DecisionRespons…

78f45bf

…e to support CMAB UUID handling

update: enhance DecisionService and DecisionMessage to handle errors …

9757d49

…and include CMAB UUIDs in responses

update: add validConfigJsonCMAB method to DatafileProjectConfigTestUt…

ecf9199

…ils for CMAB configuration

update: add tests to verify precedence of whitelisted and forced vari…

5e0808f

…ations over CMAB service decisions in DecisionService

update: add test to verify error handling in getVariation for CMAB se…

36d2b4c

…rvice failures

update: modify DecisionResponse to include additional error handling …

5796cb7

…information

update: fix error handling assertion in DecisionServiceTest to correc…

d8b0134

…tly verify error state

update: add tests for CMAB experiment variations in DecisionService

b2f270f

update: implement decision-making methods to skip CMAB logic in Optim…

a4c3f1c

…izely and DecisionService

update: add methods to OptimizelyUserContext for decision-making with…

e4fe788

…out CMAB logic

update: add asynchronous decision-making methods in OptimizelyUserCon…

e75693d

…text and related fetcher classes

update: add decision-making methods without CMAB logic in OptimizelyU…

af210d8

…serContextTest

optimizely-codereview-bot assigned FarhanAnjum-opti Sep 24, 2025

update: remove unused parameter 'useCmab' from DecisionService method…

42053e4

… documentation

muzahidul-opti requested changes Sep 25, 2025

View reviewed changes

FarhanAnjum-opti added 10 commits September 26, 2025 03:00

update: rename methods to use 'Sync' suffix for clarity in decision-m…

9a12d72

…aking logic

update: return cmab error decision whenever found

416bcbd

update: enhance error handling by specifying CMAB error messages in d…

64f378f

…ecision responses

update: improve error handling by checking for null values in experim…

8539166

…ent key retrieval

update: fix CMAB error handling by providing a valid Experiment in Fe…

3cee65c

…atureDecision

update: add Javadoc comments for async decision methods and config cr…

47c65b5

…eation in CMAB client

update: refactor build to use cmabClient instead of default service

fe75a85

update: refactor DefaultCmabClient to utilize CmabClientHelper

b0d5090

update: refactor AsyncDecisionsFetcher to AsyncDecisionFetcher and en…

6db2e88

…hance decision handling

update: add missing copyright notice and license information to CmabC…

6fc6446

…lientHelper

pvcraven requested changes Oct 3, 2025

View reviewed changes

core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientHelper.java Show resolved Hide resolved

muzahidul-opti approved these changes Oct 8, 2025

View reviewed changes

jaeopt requested changes Oct 13, 2025

View reviewed changes

jaeopt reviewed Oct 13, 2025

View reviewed changes

core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java Outdated Show resolved Hide resolved

jaeopt reviewed Oct 13, 2025

View reviewed changes

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java Outdated Show resolved Hide resolved

FarhanAnjum-opti added 5 commits October 15, 2025 18:28

update: enhance CMAB handling in bucketing and decision services, add…

a80c0d3

… backward compatibility for mobile apps

update: add backward compatibility support for Android sync and async…

a9ae805

… decisions in OptimizelyUserContext

update: add empty list parameter to decision methods in OptimizelyUse…

f25f824

…rContextTest for consistency

update: replace null with empty list parameter in async decision meth…

7363a2f

…od for consistency

update: add useCmab parameter to decideForKeys methods for enhanced d…

1c52366

…ecision handling

jaeopt requested changes Oct 17, 2025

View reviewed changes

Update core-api/src/main/java/com/optimizely/ab/Optimizely.java

b2dcf9e

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

jaeopt reviewed Oct 17, 2025

View reviewed changes

core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated Show resolved Hide resolved

FarhanAnjum-opti and others added 7 commits October 23, 2025 20:54

update: refactor decision-making logic to use DecisionPath enum for c…

89771bc

…larity and maintainability

Update core-api/src/main/java/com/optimizely/ab/Optimizely.java

73d5673

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

Update core-api/src/main/java/com/optimizely/ab/Optimizely.java

690379c

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

update: modify OptimizelyUserContext to change optimizely field to pa…

9c8dd8f

…ckage-private and add copyright notice to DecisionPath

update: implement asynchronous decision-making methods in Optimizely …

a17becd

…and OptimizelyUserContext with corresponding tests

update: refactor DefaultCmabService to remove CmabServiceOptions depe…

ba575ce

…ndency and adjust related tests

update: refactor DefaultCmabService to use a generic Cache interface …

ad474c8

…and enhance builder methods for cache configuration

jaeopt requested changes Oct 28, 2025

View reviewed changes

jaeopt and others added 7 commits October 30, 2025 14:04

fix to support android-sdk

9f7ea59

clean up

93cdaba

update: refactor bucketing logic to remove CMAB handling from Decisio…

e459321

…nService and adjust tests accordingly

update: introduce CacheWithRemove interface and refactor DefaultCmabS…

8c96222

…ervice to utilize it

update: implement CacheWithRemove interface in DefaultLRUCache class

7c97734

update: refactor OptimizelyFactory to remove CMAB cache methods and a…

4feb857

…djust instance creation logic

update: refactor DefaultCmabService to streamline logger initializati…

4b35768

…on and enhance cache handling logging

FarhanAnjum-opti requested a review from jaeopt November 6, 2025 07:35

jaeopt requested changes Nov 7, 2025

View reviewed changes

	public static final int DEFAULT_CMAB_CACHE_SIZE = 1000;
	public static final int DEFAULT_CMAB_CACHE_SIZE = 10000;

[FSSDK-11170] update: decision service methods for cmab #583

Are you sure you want to change the base?

[FSSDK-11170] update: decision service methods for cmab #583

Uh oh!

Conversation

FarhanAnjum-opti commented Sep 24, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Issues

Uh oh!

muzahidul-opti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

muzahidul-opti left a comment

Choose a reason for hiding this comment

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FarhanAnjum-opti Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FarhanAnjum-opti commented Sep 24, 2025 •

edited by atlassian bot

Loading

FarhanAnjum-opti Oct 15, 2025 •

edited

Loading